fix(shell): install nushell wrapper to vendor-autoload dir (#2878) - #2992
Conversation
The nushell integration wrote `wt.nu` to `$nu.default-config-dir/vendor/autoload`, a path Nushell never autoloads. It only worked on macOS/Windows by coincidence (there `$nu.default-config-dir == $nu.data-dir`); on Linux the wrapper was written but never loaded, so `wt` was never wrapped. Resolve the install target from `$nu.vendor-autoload-dirs | last` (the user-writable vendor-autoload dir under `$nu.data-dir`), with an XDG_DATA_HOME / `dirs::data_dir()` fallback that mirrors nushell's own `nu_path::data_dir` when `nu` isn't on PATH. The legacy `<config-dir>/vendor/autoload` locations are kept as detection candidates so install (and uninstall) clean up files stranded by older versions. - paths: query `$nu.vendor-autoload-dirs`/`$nu.default-config-dir`, build canonical + fallback + legacy candidates - install: write to the canonical vendor-autoload dir and remove worktrunk-managed wrappers stranded at legacy locations - config_line / docs: use `$nu.vendor-autoload-dirs | last | path join` - add a `WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIR` override so tests are deterministic across platforms, plus a real-`nu` guard asserting the install target is a member of `$nu.vendor-autoload-dirs` Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the data-safety branch where a legacy <config-dir>/vendor/autoload wt.nu lacking the worktrunk header is left untouched on install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
Reviewed as the PR author (self-review) — submitting as a comment, not an approval.
Data-safety surface — flagging for human review. This PR adds a new file-deletion path: cleanup_stranded_nushell in src/commands/configure_shell.rs calls fs::remove_file to delete wt.nu from <config-dir>/vendor/autoload during wt config shell install. Per the repo's deletion-surface policy, any change that introduces std::fs::remove_file is held for a maintainer rather than auto-approved.
The guard rails look right to me, for the record:
- It only removes files whose contents carry the
# worktrunk shell integration for nushellheader (is_worktrunk_managed_nushell), so a user's ownwt.nuis left alone —test_nushell_install_keeps_unmanaged_legacy_filecovers exactly this. - It never targets the canonical write path (
if &path == canonical { continue }). - A failed removal warns rather than aborting the install.
Calling it out so @max-sixty can eyeball the new deletion path before merge.
Overlaps with #2992 sibling work. PR #2864 ("validate command names and tighten install/uninstall detection") touches the same files — src/commands/configure_shell.rs, src/shell/mod.rs, src/shell/detection.rs, src/shell/paths.rs, tests/integration_tests/configure_shell.rs. Different concern, not a duplicate, but whichever lands second will need a rebase through these conflicts.
Everything else looks sound: the $nu.vendor-autoload-dirs | last resolution matches Nushell's autoload contract, the nu-absent fallback mirrors nu_path::data_dir, docs/help/snapshots are updated consistently, and the real-nu guard (test_nushell_install_target_is_a_vendor_autoload_dir) directly asserts the bug is fixed.
The new nushell install/uninstall tests failed on macOS and Windows
because the vendor-autoload override path didn't match the HOME that
`set_temp_home_env` exports (via `dunce::canonicalize`):
- macOS: tests built the override from the raw `temp_home.path()` under
`/var` (a symlink to `/private/var`), so the exported, canonicalized
HOME `/private/var/...` failed to strip-prefix it and the path printed
in full instead of `~/...` (config_show_nushell_outdated_wrapper,
uninstall_shell, uninstall_shell_multiple).
- Windows: tests used `std::fs::canonicalize`, which returns a `\\?\`
verbatim path; the forward slashes in the subsequent
`.join(".local/share/...")` are then literal filename characters, not
separators, so the wrapper write and `.exists()` check missed
(the six configure_shell nushell tests).
Add a `canonical_temp_home` helper that uses `dunce::canonicalize` — it
resolves the macOS symlink (matching the exported HOME) while returning
an ordinary, non-verbatim path on Windows — and route every nushell
override path through it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
OK, let's try this. It looks reasonable but admittedly I have not looked through each line or tested locally... I would like to be more conscientious about these; it's harder on @nnutter, great if you can test on you end, thank you... |
v0.57.0. `cargo semver-checks` reports breaking library API changes: the squash refactor (#2983) renamed `Repository::commit_subjects` to `commit_message_details`, and the new `GitError::UnbornDefaultBranch` variant (#2990) shifts the discriminants of the exhaustive `GitError` enum. Neither is intentional public API, but together they force a minor bump pre-1.0. ## CHANGELOG ### Improved - **`wt step diff --branch`**: `wt step diff` gained a `-b`/`--branch` flag, mirroring `wt step commit`, so the diff can target another worktree's branch without leaving the current one. The branch must have a checked-out worktree. ([#2995](#2995)) - **Squash templates can use commit bodies**: The squash commit-message template gains an experimental `{{ commit_details }}` variable — a list of `{ subject, body }` objects for the commits being squashed — alongside the existing `{{ commits }}` (now documented as the commit subjects). Templates can incorporate full commit bodies, not just subject lines. ([#2983](#2983), thanks @florianilch) - **Recommended Claude Code commit command drops the `CLAUDECODE=` prefix**: Claude Code removed the nested-session check that rejected `claude -p` launched from inside another session, so the workaround is gone. The recommended `[commit.generation]` command shown by `wt config create` no longer carries a leading `CLAUDECODE=`, and `wt` no longer strips `CLAUDECODE` from the environment before running commit-generation commands. ([#2979](#2979)) ### Fixed - **Nushell wrapper installs where Nushell actually autoloads it**: `wt config shell install nu` wrote `wt.nu` to `$nu.default-config-dir/vendor/autoload`, which Nushell never autoloads — on Linux the wrapper was written but silently never loaded, so `wt` was never wrapped (it happened to work on macOS/Windows by coincidence of path layout). It now installs to `$nu.vendor-autoload-dirs | last`, and install/uninstall clean up any worktrunk wrapper stranded at the old location. ([#2992](#2992), thanks @nnutter for reporting) - **Claude Code hooks work for Fish shell users**: The plugin's hook commands used `${CLAUDE_PLUGIN_ROOT}` brace syntax, which Fish doesn't expand; they now use `$CLAUDE_PLUGIN_ROOT`, so the activity and worktree-lifecycle hooks fire correctly under Fish. ([#2962](#2962), thanks @amw) - **Pager no longer wedges the terminal on Ctrl-C (Windows)**: Interrupting the `--help` pager (`less`) with Ctrl-C on Windows could leave the terminal in a broken state; `less` now quits cleanly on interrupt. ([#2969](#2969), thanks @ofek for reporting) - **Clearer error when the default branch has no commits**: In a freshly initialized repo whose default branch is unborn, `merge`/`rebase`/`squash`/`push` (and the diff report) failed with `Default branch main does not exist locally` plus a misleading hint to reset the cached value. They now report that the branch has no commits yet, without the wrong cache-reset suggestion. ([#2990](#2990)) - **`diagnostic.md` uploads as a gist again**: The `-vv` diagnostic report inlined raw NUL bytes from NUL-separated git output, so `gh gist create` rejected it as a binary file. Control bytes in the subprocess preview are now escaped. ([#2991](#2991)) - **`wt list` tolerates a missing index file**: A repo with no `<gitdir>/index` (nothing ever staged) made the temp-index probe fail; a missing index is now treated as an empty one, matching git's own behavior. ([#2884](#2884)) - **Inline code renders in `--help` section headings**: Terminal `--help` showed literal backticks in headings authored with inline code (e.g. the `wt config state logs` heading `Command log (`commands.jsonl`)`). Headings now reduce inline code to plain text under the heading's uniform style. The `--stage`/`--dry-run` subsection headings in `wt step commit`/`squash` were also renamed to sentence case ("Staging", "Dry run"). ([#3003](#3003)) ### Documentation - **`wt switch` docs give forge PR/MR URLs equal billing with `pr:`/`mr:`**: The switch docs now present the full forge-URL form alongside the `pr:N` shortcut. ([#2970](#2970)) - **New FAQ entry on moving uncommitted changes to a new worktree**. ([#3002](#3002)) ### Internal - **Bare-repo prompt opt-out stored as a hint**: `worktrunk.skip-bare-repo-prompt` moved under the `worktrunk.hints.` namespace, so it now lists under `wt config state hints` and clears with `wt config state clear` (previously a top-level key that escaped both). Clean cutover: users who already opted out are re-prompted once on their next `wt switch --create` in a dotted-name bare repo. ([#3001](#3001)) > _This was written by Claude Code on behalf of max_
…em (#3602) `wt config shell install` decided whether the file at a legacy location was worktrunk's by reading it — a substring test for the wrapper header, falling back to "every code line looks like an integration line". Anything that didn't match was left in place. Fish sources `conf.d` at startup, so whatever `conf.d/{cmd}.fish` defines is already loaded by the time fish would autoload the `functions/{cmd}.fish` the install just wrote — the stale definition wins and the new wrapper may never load. Ownership is now the path. `conf.d/{cmd}.fish` and the stranded nushell `{cmd}.nu` candidates are paths worktrunk itself computes for the command name being installed, so install takes them back whole without reading them. Only that exact filename is touched: a neighbour under another name is not worktrunk's, and each removal is still reported. `wt config shell uninstall` still reads the header, because it takes no `--cmd` and so genuinely doesn't know the name — it lists the shell-owned directories and has to tell our `{cmd}.fish` from the user's own files beside it. That is the one place ownership can't come from the path, and it prompts and previews every file before removing it, which install does neither of. `is_worktrunk_managed_content`'s docstring now says so; `is_worktrunk_managed_nushell` existed only to gate the install-time deletion and is gone. Audited the rest of the surface for the same principle: the OpenCode plugin (`~/.config/opencode/plugins/worktrunk.ts`) is already removed by path with no inspection, the Claude plugin delegates to `claude plugin uninstall`, Gemini installs through `gemini extensions install` and leaves nothing of ours on disk, and fish is the only shell with a separate completion file — which uninstall already covers. No other gaps. ## Data safety This deliberately widens what install deletes, so it's worth naming: two tests asserted the old conservatism and now assert the new boundary — `test_configure_shell_fish_reclaims_conf_d_path` (was `..._preserves_user_conf_d_file`, from #3589) and `test_nushell_install_reclaims_only_the_command_name` (was `..._keeps_unmanaged_legacy_file`, from #2992). Both put a neighbour file in the same directory to pin that only `{cmd}.{ext}` is taken. The FAQ's "What can Worktrunk delete?" inventory is updated to describe path-based ownership rather than the old content-marker rule. > _This was written by Claude Code on behalf of max_ Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes #2878.
Implements the approach agreed in the issue: query
$nu.vendor-autoload-dirsdirectly and install to its last (user-writable) entry, with cross-platform tests and stranded-file cleanup on install/uninstall.The bug
The nushell integration wrote
wt.nuto$nu.default-config-dir/vendor/autoload, which Nushell never autoloads. It happened to work on macOS/Windows because there$nu.default-config-dir == $nu.data-dir, so the wrong subpath resolved to the real vendor dir; on Linux the wrapper was written but silently never loaded — sowtwas never wrapped.Verified against
nu0.113.1:$nu.vendor-autoload-dirs | lastis<data-dir>/vendor/autoload, and$nu.default-config-dir/vendor/autoloadis in neither$nu.vendor-autoload-dirsnor$nu.user-autoload-dirs.The fix
$nu.vendor-autoload-dirs | last(one memoisednuspawn that also reads$nu.default-config-dirfor legacy cleanup).nuisn't on PATH mirrors nushell's ownnu_path::data_dir:XDG_DATA_HOME(when absolute) wins on every platform, otherwisedirs::data_dir()— matching nushell on Linux/macOS/Windows.paths.first()), so a wrapper stranded at a legacy path can't become the write target. (No-op for fish, which has a single canonical path.)<config-dir>/vendor/autoloadlocations; uninstall already iterates every candidate, which now includes those legacy paths. Legacy files are only removed when they carry the# worktrunk shell integration for nushellheader, so a user's ownwt.nuis never touched.config_line, thewt config shell initmanual-setup help, and the FAQ/shell-integration docs now use$nu.vendor-autoload-dirs | last | path join wt.nu.Tests
WORKTRUNK_TEST_NU_VENDOR_AUTOLOAD_DIRoverride pins the install dir so the install/uninstall/cleanup tests are deterministic on Linux, macOS, and Windows and don't depend onnubeing installed.test_nushell_install_target_is_a_vendor_autoload_dir(gated onshell-integration-tests, where CI hasnu) runs the realnuflow and asserts the wrapper worktrunk wrote is a member of$nu.vendor-autoload-dirs. This fails againstmain(the old config-dir path is in neither autoload list) and passes with the fix.test_nushell_install_cleans_stranded_legacyand a reworkedtest_uninstall_nushell_cleans_all_candidate_locationscover the migration cleanup.cargo test --lib+cargo test --test integration, and the nushell subset with realnuunder--features shell-integration-tests).Not in scope
"Not best practice" cleanup flagged in the issue (the overlap between the candidate-building helpers, and the dubious Windows branch) — the helpers here are restructured around the autoload model, but a broader tidy can be a follow-up.
🤖 Generated with Claude Code